Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib: add a keyboard layout per window script #4504

Closed

Conversation

emersion
Copy link
Member

Fixes #2361

@emersion emersion mentioned this pull request Aug 27, 2019
@progandy
Copy link
Contributor

progandy commented Aug 27, 2019

I believe stored layouts should be removed on window::close and prev_focused cleared if it references the closed window.

By the way, does this work with multiple seats?

import i3ipc

ipc = i3ipc.Connection()
prev_focused = -1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use current window id ipc.get_tree().find_focused().id?

# Save current layout
layouts = {}
for input in ipc.get_inputs():
layouts[input.identifier] = input.xkb_active_layout_index
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating layouts uninitialized is unnecessary here, you can create them initialized as:

    layouts = {input.identifier : input.xkb_active_layout_index
               for input in ipc.get_inputs()}

And it's faster as a bonus.

@emersion emersion force-pushed the contrib-kb-layout-per-window branch from 048aada to 2acee22 Compare September 1, 2019 19:59
@emersion
Copy link
Member Author

emersion commented Sep 2, 2019

By the way, does this work with multiple seats?

No

@Hi-Angel
Copy link
Contributor

Hi-Angel commented Sep 2, 2019

By the way, does this work with multiple seats?

No

If we consider this PR as a replacement to #3155, then per-seat functional needs to be implemented, because, clearly, the replacement needs to be at least as good as the one supposed to get replaced.

@eternal-sorrow
Copy link

eternal-sorrow commented Sep 2, 2019

I think, this script isn't supposed to be the catch-all solution for everyone. Personally, I'm going to create another one that saves the layout data on disk and restores it after restart and also has some other nice features (uses asyncio with i3ipc-python instead of the old API for example).

@emersion
Copy link
Member Author

emersion commented Sep 3, 2019

the replacement needs to be at least as good as the one supposed to get replaced.

This is not supposed to be a replacement. This is supposed to be an alternate solution. To be honest, I won't even be using it myself, the goal here is just to give a hint for people who want this feature. Feel free to improve it if you want multi-seat or other features.

Full disclaimer: I don't personally care if this gets merged or not. But if you're so salty about it, I'd suggest to completely ignore this PR.

@Hi-Angel
Copy link
Contributor

Hi-Angel commented Sep 3, 2019

Full disclaimer: I don't personally care if this gets merged or not. But if you're so salty about it, I'd suggest to completely ignore this PR.

I don't understand what you mean. I didn't say anything wrong or bad, did I? My assumption that this PR was supposed to be a replacement didn't appear out of pure air, it were your words #3155 (comment), quoting:

Is this moving any further?

#4504

I'm sorry if I was confused, and these 2 PRs supposed to co-exist. Although I wonder then, why would you post this reply (the one I quoted).

@emersion
Copy link
Member Author

emersion commented Sep 3, 2019

Sorry, I've had enough of this. I'm dropping the ball.

@Hi-Angel

This comment has been minimized.

@NEOhidra

This comment has been minimized.

if event.container.id in windows:
for (input_id, layout_index) in windows[event.container.id].items():
if layout_index != layouts[input_id]:
ipc.command("input \"{}\" xkb_switch_layout {}".format(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ipc.command("input \"{}\" xkb_switch_layout {}".format(
ipc.command("""input "{}" xkb_switch_layout {}""".format(

maybe this is more readable, the back slashes really threw me off

@ddevault ddevault closed this Sep 18, 2019
@swaywm swaywm locked and limited conversation to collaborators Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard layout-per-window feature
7 participants